Update the plugin to be Ruby 2.5+ and Rubocop

.rubocop.yml

@@ -1,16 +1,20 @@
 inherit_from: .rubocop_todo.yml
 
+require:
+  - rubocop-performance
+  - rubocop-rake
+
 AllCops:
-  TargetRubyVersion: 2.0
+  NewCops: enable
+  TargetRubyVersion: 2.5
   Exclude:
     - 'test/**/*'
     - 'vendor/**/*'
 
 Metrics:
   Enabled: false
 
-# There's if statement in the Gemfile
-Bundler/DuplicatedGem:
+Gemspec/RequireMFA:
   Enabled: false
 
 Layout/LeadingCommentSpace:
@@ -113,10 +117,10 @@ Style/EmptyMethod:
 Style/ParenthesesAroundCondition:
   Enabled: false
 
-Layout/AlignHash:
+Layout/HashAlignment:
   Enabled: false
 
-Layout/AlignParameters:
+Layout/ParameterAlignment:
   Enabled: false
 
 # disabled until we can configure "+" as concat sign

Gemfile

@@ -1,21 +1,20 @@
 source 'https://rubygems.org'
 gemspec
 
-if RUBY_VERSION < '2.0.0'
-  gem 'json', '< 2.0.0', :require => false
-end
-
-group :development do
-  gem 'rake'
-  gem 'test-unit'
+gem 'webrick'
 
-  if RUBY_VERSION < '2.2.2'
-    gem 'rack-test', '~> 0.7.0'
-  else
-    gem 'rack-test'
-  end
+group :rubocop do
+  gem 'rubocop', '~> 1.28.0'
+  gem 'rubocop-performance'
+  gem 'rubocop-rake'
+end
 
+group :test do
+  gem 'ci_reporter_test_unit'
   gem 'concurrent-ruby', '~> 1.0', require: 'concurrent'
   gem 'mocha'
+  gem 'rack-test'
+  gem 'rake'
   gem 'smart_proxy', :github => 'theforeman/smart-proxy', :branch => 'develop'
+  gem 'test-unit'
 end

Rakefile

@@ -1,3 +1,4 @@
+require 'ci/reporter/rake/test_unit'
 require 'rake'
 require 'rake/testtask'
 require 'rubocop/rake_task'
@@ -15,3 +16,8 @@ Rake::TestTask.new(:test) do |t|
   t.test_files = FileList['test/**/*_test.rb']
   t.verbose = true
 end
+
+namespace :jenkins do
+  desc nil # No description means it's not listed in rake -T
+  task :unit => ['ci:setup:testunit', :test]
+end

lib/smart_proxy_dhcp_infoblox/common_crud.rb

@@ -19,6 +19,7 @@ def all_leases(network_address, subnet)
       ::Infoblox::Lease.find(@connection, 'address~' => address_range_regex).map do |lease|
         # Infoblox can return MAC address set to nil
         next unless lease.hardware
+
         Proxy::DHCP::Lease.new(
           lease.client_hostname,
           lease.address,
@@ -34,6 +35,7 @@ def all_leases(network_address, subnet)
 
     def find_record(subnet_address, an_address)
       return find_record_by_ip(subnet_address, an_address) if Resolv::IPv4::Regex =~ an_address
+
       find_record_by_mac(subnet_address, an_address)
     end
 
@@ -58,26 +60,31 @@ def add_record(options)
       if options[:mac] != existing_host.mac || options[:hostname] != existing_name
         raise Proxy::DHCP::Collision, "Record #{options[:ip]} conflicts with an existing record."
       end
+
       raise Proxy::DHCP::AlreadyExists, "Record #{options[:ip]} already exists."
     end
 
     def del_record(_, record)
       raise InvalidRecord, "#{record} is static - unable to delete" unless record.deleteable?
+
       found = find_hosts('ipv4addr' => record.ip).first
       return if found.nil?
+
       found.delete
     end
 
     def del_records_by_ip(ip_address)
       found = find_hosts({ 'ipv4addr' => ip_address }, 2147483646)
       return if found.empty?
+
       found.each { |record| record.delete }
       nil
     end
 
     def del_record_by_mac(mac_address)
       found = find_hosts('mac' => mac_address).first
       return if found.nil?
+
       found.delete
       nil
     end
@@ -90,11 +97,13 @@ def build_reservation(name, host, full_subnet_address)
 
       opts = { :hostname => name }
       opts[:deleteable] = true
-      # TODO: nextserver, use_nextserver, bootfile, and use_bootfile attrs exist but are not available in the Fixedaddress model
+      # TODO: nextserver, use_nextserver, bootfile, and use_bootfile attrs exist
+      # but are not available in the Fixedaddress model
       # Might be useful to extend the model to include these
       opts[:nextServer] = host.nextserver if (host.respond_to?(:use_nextserver) && host.use_nextserver)
       opts[:filename] = host.bootfile if (host.respond_to?(:use_bootfile) && host.use_bootfile)
-      subnet = ::Proxy::DHCP::Subnet.new(full_subnet_address.split('/').first, cidr_to_ip_mask(cidr_to_i(full_subnet_address.split('/').last)))
+      subnet = ::Proxy::DHCP::Subnet.new(full_subnet_address.split('/').first,
+                                         cidr_to_ip_mask(cidr_to_i(full_subnet_address.split('/').last)))
 
       Proxy::DHCP::Reservation.new(name, host.ipv4addr, host.mac, subnet, opts)
     end

lib/smart_proxy_dhcp_infoblox/dhcp_infoblox_main.rb

@@ -93,6 +93,7 @@ def find_network(network_address)
       network = ::Infoblox::Network.find(connection, 'network' => network_address, 'network_view' => network_view,
                                          '_max_results' => 1).first
       raise "Subnet #{network_address} not found in network view #{network_view}" if network.nil?
+
       network
     end
 

lib/smart_proxy_dhcp_infoblox/dhcp_infoblox_plugin.rb

@@ -3,11 +3,11 @@ class Plugin < ::Proxy::Provider
     plugin :dhcp_infoblox, ::Proxy::DHCP::Infoblox::VERSION
 
     default_settings :record_type => 'fixedaddress',
-        :dns_view => "default",
-        :network_view => "default",
-        :blacklist_duration_minutes => 30 * 60,
-        :wait_after_restart => 10,
-        :options => []
+                     :dns_view => "default",
+                     :network_view => "default",
+                     :blacklist_duration_minutes => 30 * 60,
+                     :wait_after_restart => 10,
+                     :options => []
 
     validate_presence :username, :password
 

lib/smart_proxy_dhcp_infoblox/fixed_address_crud.rb

@@ -21,24 +21,28 @@ def all_hosts(subnet_address)
     def find_record_by_ip(subnet_address, ip_address)
       found = find_hosts('ipv4addr' => ip_address).first
       return nil if found.nil?
+
       build_reservation(found.name, found, subnet_address)
     end
 
     def find_records_by_ip(subnet_address, ip_address)
       found = find_hosts({ 'ipv4addr' => ip_address }, 2147483646)
       return [] if found.empty?
+
       to_return = found.map { |record| build_reservation(record.name, record, subnet_address) }
       to_return.compact
     end
 
     def find_record_by_mac(subnet_address, mac_address)
       found = find_hosts('mac' => mac_address).first
       return nil if found.nil?
+
       build_reservation(found.name, found, subnet_address)
     end
 
     def find_hosts(condition, max_results = 1)
       return @memoized_hosts if (!@memoized_hosts.empty? && @memoized_condition = condition)
+
       @memoized_condition = condition
       @memoized_hosts = ::Infoblox::Fixedaddress.find(@connection, condition.merge('_max_results' => max_results,
                                                                                    'network_view' => network_view))

lib/smart_proxy_dhcp_infoblox/host_ipv4_address_crud.rb

@@ -17,30 +17,36 @@ def all_hosts(subnet_address)
 
       hosts = ::Infoblox::Host.find(
         @connection,
-          'ipv4addr~' => address_range_regex,
-          'view' => dns_view,
-          '_max_results' => 2147483646)
+        'ipv4addr~' => address_range_regex,
+        'view' => dns_view,
+        '_max_results' => 2147483646)
 
       ip_addr_matcher = Regexp.new(address_range_regex) # pre-compile the regex
-      hosts.map { |host| build_reservation(host.name, host.ipv4addrs.find { |ip| ip_addr_matcher =~ ip.ipv4addr }, subnet_address) }.compact
+      hosts.map do |host|
+        build_reservation(host.name, host.ipv4addrs.find { |ip| ip_addr_matcher =~ ip.ipv4addr }, subnet_address)
+      end.compact
     end
 
     def find_record_by_ip(subnet_address, ip_address)
       found = find_hosts('ipv4addr' => ip_address).first
       return nil if found.nil?
+
       build_reservation(found.name, found.ipv4addrs.find { |ip| ip.ipv4addr == ip_address }, subnet_address)
     end
 
     def find_records_by_ip(subnet_address, ip_address)
       found = find_hosts({ 'ipv4addr' => ip_address }, 2147483646)
       return [] if found.empty?
-      to_return = found.map { |record| build_reservation(record.name, record.ipv4addrs.find { |ip| ip.ipv4addr == ip_address }, subnet_address) }
-      to_return.compact
+
+      found.map do |record|
+        build_reservation(record.name, record.ipv4addrs.find { |ip| ip.ipv4addr == ip_address }, subnet_address)
+      end.compact
     end
 
     def find_record_by_mac(subnet_address, mac_address)
       found = find_hosts('mac' => mac_address).first
       return nil if found.nil?
+
       build_reservation(found.name, found.ipv4addrs.find { |ip| ip.mac == mac_address }, subnet_address)
     end
 
@@ -51,6 +57,7 @@ def find_host_and_name_by_ip(ip_address)
 
     def find_hosts(condition, max_results = 1)
       return @memoized_hosts if (!@memoized_hosts.empty? && @memoized_condition == condition)
+
       @memoized_condition = condition
       @memoized_hosts = ::Infoblox::Host.find(@connection, condition.merge('view' => dns_view,
                                                                            '_max_results' => max_results))

lib/smart_proxy_dhcp_infoblox/ip_address_arithmetic.rb

@@ -1,20 +1,20 @@
 module ::Proxy::DHCP::Infoblox
   module IpAddressArithmetic
     def cidr_to_ip_mask(prefix_length)
-      bitmask = 0xFFFFFFFF ^ (2**(32 - prefix_length) - 1)
-      (0..3).map { |i| (bitmask >> i * 8) & 0xFF }.reverse.join('.')
+      bitmask = 0xFFFFFFFF ^ ((2**(32 - prefix_length)) - 1)
+      (0..3).map { |i| (bitmask >> (i * 8)) & 0xFF }.reverse.join('.')
     end
 
     def ipv4_to_i(an_address)
       an_address.split('.').inject(0) { |a, c| (a << 8) + c.to_i }
     end
 
-    def i_to_ipv4(i)
+    def i_to_ipv4(i) # rubocop:todo Naming/MethodParameterName
       (0..3).inject([]) { |a, c| a.push((i >> (c * 8)) & 0xFF) }.reverse.join('.')
     end
 
     def cidr_to_bitmask(prefix_length)
-      0xFFFFFFFF ^ (2**(32 - prefix_length) - 1)
+      0xFFFFFFFF ^ ((2**(32 - prefix_length)) - 1)
     end
 
     def cidr_to_i(an_address_with_cidr)

lib/smart_proxy_dhcp_infoblox/network_address_range_regex_generator.rb

@@ -12,6 +12,7 @@ def initialize(value, children = [])
 
       def add_children(values)
         return if values.empty?
+
         node = (found = children.find { |n| n.value == values.first }).nil? ? add_child(Node.new(values.first)) : found
         node.add_children(values[1..-1])
       end
@@ -21,6 +22,7 @@ def <=>(other)
         return 1 if other.value.to_s == '0?'
         return 0 if value == other.value
         return -1 if value < other.value
+
         1
       end
 
@@ -33,9 +35,13 @@ def add_child(a_node)
       def group_children
         children.each { |n| n.group_children }
         return if children.size < 2
+
         @children = children[1..-1].each_with_object([MergedNode.new(children.first)]) do |to_group, grouped|
           current = MergedNode.new(to_group)
-          found = grouped.find { |g| ((g.value != ['0?'] && current.value != ['0?']) || (current.value == ['0?'] && g.value == ['0?'])) && (g.children == current.children) }
+          found = grouped.find do |g|
+            ((g.value != ['0?'] && current.value != ['0?']) || (current.value == ['0?'] && g.value == ['0?'])) && \
+              (g.children == current.children)
+          end
           found.nil? ? grouped.push(current) : found.merge(current)
         end
       end
@@ -47,6 +53,7 @@ def as_regex
 
     class MergedNode
       attr_accessor :value, :children
+
       def initialize(a_node)
         @value = [a_node.value].flatten
         @children = a_node.children
@@ -62,11 +69,12 @@ def as_regex
       end
 
       def value_as_regex
-        (value.size < 2) ? value.first.to_s : "[#{value.join('')}]"
+        (value.size < 2) ? value.first.to_s : "[#{value.join}]"
       end
 
       def ==(other)
         return false if self.class != other.class
+
         value == other.value
       end
     end

lib/smart_proxy_dhcp_infoblox/plugin_configuration.rb

@@ -10,11 +10,12 @@ def load_classes
       require 'smart_proxy_dhcp_infoblox/dhcp_infoblox_main'
     end
 
-    def load_dependency_injection_wirings(c, settings)
+    def load_dependency_injection_wirings(c, settings) # rubocop:todo Naming/MethodParameterName
       c.dependency :connection, (lambda {
         ::Infoblox.wapi_version = '2.0'
         ::Infoblox::Connection.new(:username => settings[:username], :password => settings[:password],
-                                   :host => settings[:server], :ssl_opts => { :verify => !ENV['FOREMAN_INFOBLOX_NOSSLVERIFY'] },
+                                   :host => settings[:server],
+                                   :ssl_opts => { :verify => !ENV['FOREMAN_INFOBLOX_NOSSLVERIFY'] },
                                    :logger => ::Proxy::LogBuffer::Decorator.instance)
       })
 
@@ -30,7 +31,7 @@ def load_dependency_injection_wirings(c, settings)
       c.dependency :dhcp_provider, (lambda {
         ::Proxy::DHCP::Infoblox::Provider.new(
           c.get_dependency(:connection),
-          (settings[:record_type] == 'host') ? c.get_dependency(:host_ipv4_crud) : c.get_dependency(:fixed_address_crud),
+          c.get_dependency(settings[:record_type] == 'host' ? :host_ipv4_crud : :fixed_address_crud),
           c.get_dependency(:grid_restart),
           c.get_dependency(:unused_ips),
           settings[:subnets],

smart_proxy_dhcp_infoblox.gemspec

@@ -1,10 +1,8 @@
-require File.expand_path('../lib/smart_proxy_dhcp_infoblox/dhcp_infoblox_version', __FILE__)
-require 'date'
+require File.expand_path('lib/smart_proxy_dhcp_infoblox/dhcp_infoblox_version', __dir__)
 
 Gem::Specification.new do |s|
   s.name        = 'smart_proxy_dhcp_infoblox'
   s.version     = Proxy::DHCP::Infoblox::VERSION
-  s.date        = Date.today.to_s
   s.license     = 'GPL-3.0'
   s.authors     = ['Klaas Demter']
   s.email       = ['[email protected]']
@@ -16,7 +14,7 @@ Gem::Specification.new do |s|
   s.files       = Dir['{config,lib,bundler.d}/**/*'] + ['README.md', 'LICENSE']
   s.test_files  = Dir['test/**/*']
 
-  s.add_runtime_dependency('infoblox', '~> 3.0')
+  s.required_ruby_version = '>= 2.5'
 
-  s.add_development_dependency('rubocop', '~> 0.50.0')
+  s.add_runtime_dependency('infoblox', '~> 3.0')
 end

test/test_helper.rb

@@ -1,5 +1,5 @@
 require 'test/unit'
-require 'mocha/setup'
+require 'mocha/test_unit'
 
 require 'smart_proxy_for_testing'